-
-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate LLD for MSVC targets; prepare for other targets #2142
Conversation
94d5ce6
to
e0b34e2
Compare
…_LLD Results in a 7.5% bigger RelWithDebInfo ldc2.exe on Win64 with LLVM 3.9.1. LLD is currently enforced when building with LDC_WITH_LLD=ON. And LLD still doesn't support debuginfo (.pdb) generation for MSVC targets.
Alright, I just tested directly cross-compiling and -linking from Linux x64 to Win64. It works without much hassle.
You'll then be able to produce Win64 executables and DLLs by simply using the |
This also means that if we are allowed to redistribute the MS libs, we could provide a self-sufficient package for Windows, similar to the DMD package, without relying on any Visual C++ installation (and well basically depending on nothing at all except for Windows these days - no libconfig, no shared C runtime, no external linker & libs, ...). A self-sufficient Win64-only package (no 32-bit libs) would be a ~50 MB download, just to give you an idea. |
Nice! Do you know what the licensing situation is on the MS libraries? |
How big is the impact of |
Nope, no idea.
Size-wise for the |
I got rid of |
driver/linker.cpp
Outdated
staticFlag("static", llvm::cl::ZeroOrMore, | ||
llvm::cl::desc("Create a statically linked binary, including " | ||
"all system dependencies")); | ||
|
||
#if LDC_WITH_LLD | ||
static llvm::cl::opt<bool> | ||
useInternalLinker("internal-linker", llvm::cl::ZeroOrMore, llvm::cl::Hidden, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe -link-internally
or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed.
if(MSVC) | ||
list(APPEND LDC_LINKERFLAG_LIST lldCOFF.lib lldCore.lib lldDriver.lib) | ||
else() | ||
set(LDC_LINKERFLAG_LIST "-llldCOFF;-llldCore;-llldDriver;${LDC_LINKERFLAG_LIST}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an equivalent of llvm-config
for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I've seen, in fact I didn't find anything about how to embed LLD via CMake. On Linux the order matters (even inbetween these 3 new libs), so I put them in front. As the LLD libs don't have the LLVM
prefix, we can't specify them in find_package()
...
CMakeLists.txt
Outdated
# | ||
set(LDC_WITH_LLD OFF) | ||
if(LDC_LLVM_VER GREATER 308) | ||
if(EXISTS "${LLVM_INCLUDE_DIRS}/lld/Driver/Driver.h") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just use the standard CMake header search functionality? ${LLVM_INCLUDE_DIRS}/…
(with the plural) looks quite wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CHECK_INCLUDE_FILE()
didn't work (although there's a previous line with include_directories(SYSTEM ${LLVM_INCLUDE_DIRS})
- no extra include dirs appear to be used for the check).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does CMAKE_REQUIRED_INCLUDES do the trick?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm it doesn't for Travis (LLVM 4.0). :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gcc needs an additional -std=c++11
flag; fixed now.
Edit: Dammit, clang (Travis OSX) still fails.
Edit2: Works with clang 3.8 here in my VM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, I missed that the first Travis OSX job now uses the SPIRV LLVM which apparently doesn't feature LLD. So all good, LLD is dectected correctly for the other job.
driver/linker-msvc.cpp
Outdated
} | ||
|
||
for (unsigned i = 0; i < global.params.linkswitches->dim; i++) { | ||
addSwitch(global.params.linkswitches->data[i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably code that existed before, but you can use (*global.params.linkswitches)[i]
or our range-based for wrapper.
Yesterday I tried integrating LLD for ELF and Mach-O targets as well. Sadly, there's an unknown number of conflicting command-line options then (which only show up after linking, when running LDC; additionally, only the first conflicting option is shown as I had to find out). I renamed the Oh and the ldc2.exe size grew by about 20% compared to LDC without LLD at all. |
And infer CMake LDC_WITH_LLD automatically based on availability of LLVM 3.9+ with LLD headers & libs.
(about cmdline options: I think it'd be best to someday go the Clang route: put all LLVM options behind a |
Very cool work btw! |
The problem is that LLD's ELF/LTO.cpp drags in an LLVM header with quite a number of command-line options, quite a few colliding with our duplicates, but also many new options; at least some would also make sense for LDC. The include has been removed in LLD master, but it's there for 3.9 and 4.0. |
I quickly looked into using that shared LLVM header ourselves (in a separate .cpp, so that it's only pulled in if it's not already via LLD), but what's required is a hack. The actually required shared utility functions (setting up a TargetMachine's TargetOptions according to all codegen command-line options etc. - very handy) are all marked as |
7533746
to
2acddf9
Compare
licensing or you could just include a little call to download and install vcredist* as a script in the repo. (older versions usually require logging in with MS account, unfortunately, but curl should pick up latest one ). I don't know if vcredist installer works on wine. |
We don't need to redistribute the DLLs (easy enough to install and only needed when running binaries created with non-default |
They do support static linking, so one would think that their license supported some form of redistribution. |
They allow redistribution of libraries after they are run through linker :) |
No description provided.